Skip to content

Add passkey tests verifying WebAuthn conformance #62441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

MackinnonBuck
Copy link
Member

Add passkey tests verifying WebAuthn conformance

Adds tests to verify that the ASP.NET Core Identity passkey implementation conforms to the WebAuthn standard.

Description

We have an existing sample project in this repo (IdentitySample.PasskeyConformance) that can be run against the official FIDO Conformance Testing tools to verify that our implementation of passkeys is conformant with the WebAuthn specification. However, it's currently not possible to run this tool in CI, as the tool is not open source nor does it have a CLI interface.

This PR adds CI-compatible tests that verify conformance with the WebAuthn specification, at least to the degree that we currently support (e.g., there are no attestation statement verification tests). Instead of validating passkey functionality in an E2E manner like the conformance testing tool (we already have some coverage in existing E2E tests), these tests directly validate the DefaultPasskeyHandler implementation in-memory. This is a simpler approach that gives us full control over test input data that would normally be generated by the browser and allows us to validate implementation-specific behavior that the official conformance testing tool cannot.

This PR also extends existing functional tests to cover passkey functionality:

  • Tests for version 3 of the Identity store schema
  • New SignInManager tests
  • New UserManager tests

Fixes #62440

@MackinnonBuck
Copy link
Member Author

Note that this PR currently targets #62112, which hasn't been merged because it's blocked on #62356. After those PRs get merged, I'll change the base of this PR to main.

@MackinnonBuck MackinnonBuck requested a review from halter73 June 23, 2025 16:09
@gfoidl gfoidl added the area-identity Includes: Identity and providers label Jun 24, 2025
Base automatically changed from mbuck/passkeys to main June 25, 2025 17:35
@MackinnonBuck MackinnonBuck requested review from a team and wtgodbe as code owners June 25, 2025 17:35
@MackinnonBuck MackinnonBuck force-pushed the mbuck/passkey-follow-ups branch from cc9262a to bdb2262 Compare June 25, 2025 18:15
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds CI-compatible tests that verify conformance with the WebAuthn specification, at least to the degree that we currently support (e.g., there are no attestation statement verification tests). Instead of validating passkey functionality in an E2E manner like the conformance testing tool (we already have some coverage in existing E2E tests), these tests directly validate the DefaultPasskeyHandler implementation in-memory. This is a simpler approach that gives us full control over test input data that would normally be generated by the browser and allows us to validate implementation-specific behavior that the official conformance testing tool cannot.

This looks really good. I agree with the in-memory approach. Other than the attestation statement verification tests, is there any other coverage that we're missing? Feel free to ignore my comment about the file structure. Only change it if you think it's better.

}

// Represents a test scenario for a passkey operation (attestation or assertion)
private abstract class PasskeyTestBase<TResult>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see someone else is not a fan of XUnit test fixtures either 😆. Would it be simpler to put the helper types like this and CredentialKeyPair in their own files in a new Identity.Test/Passkeys subfolder rather than having a huge DefaultPasskeyHandlerTest split up over four files?

I'm fine with keep the attestation and authentication tests in separate files, but I think they should have different class names. That probably means a lot of private stuff will need to be made internal/public that won't be needed elsewhere, but at least it'd all be the same namespace.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be simpler to put the helper types like this and CredentialKeyPair in their own files in a new Identity.Test/Passkeys subfolder rather than having a huge DefaultPasskeyHandlerTest split up over four files?

It might be - I did at one point try moving some of these helpers out into non-nested types, but that was at the same time I attempted an insane refactor that abstracted similar tests between attestation and assertion into a common base test class, which ended up being a bad idea. I'll try "just" moving these into separate files and having attestation and assertion tests be in separate classes to see if that works better 🙂

@MackinnonBuck
Copy link
Member Author

Other than the attestation statement verification tests, is there any other coverage that we're missing?

Not that I'm aware of - I think these tests cover everything else that the conformance testing tool checks for (plus things it doesn't/couldn't check for, such as handling incorrectly formatted creation/request options, attempting to log in with a credential ID not associated with the user identified prior to the login ceremony, etc.).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-identity Includes: Identity and providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants